Add sdJwt presentation #4

Pflanzmann · 2024-08-14T07:36:39Z

No description provided.

FelixRedAndroid

Looks good! Still left a couple of comments regarding unused code and stuff.

FelixRedAndroid · 2024-08-14T10:55:00Z

wallet-core/src/main/java/eu/europa/ec/eudi/wallet/EudiWallet.kt

+
+            DocumentManagerSdJwt
+                .getAllDocuments()
+//                .filter { doc -> doc.vct == req.docType }


Uncommented code, remove?

FelixRedAndroid · 2024-08-14T10:55:06Z

wallet-core/src/main/java/eu/europa/ec/eudi/wallet/EudiWallet.kt

                .filterIsInstance<IssuedDocument>()
-                .filter { doc -> doc.docType == req.docType }
+//                .filter { doc -> doc.docType == req.docType }


Uncommented code, remove?

FelixRedAndroid · 2024-08-14T10:55:19Z

wallet-core/src/main/java/eu/europa/ec/eudi/wallet/issue/openid4vci/DefaultOffer.kt

+            //.filterKeys { it in credentialOffer.credentialConfigurationIdentifiers }
+            //.filterValues { credentialConfigurationFilter(it) }


Uncommented code, remove?

Still uncommented.

Hab einen Kommentar drüber damit man weiß, dass das wieder rein muss

FelixRedAndroid · 2024-08-14T10:56:54Z

wallet-core/src/main/java/eu/europa/ec/eudi/wallet/issue/openid4vci/OfferResolver.kt

@@ -20,6 +20,7 @@ import eu.europa.ec.eudi.openid4vci.CredentialOfferRequestResolver
 import eu.europa.ec.eudi.wallet.issue.openid4vci.CredentialConfigurationFilter.Companion.Compose
 import eu.europa.ec.eudi.wallet.issue.openid4vci.CredentialConfigurationFilter.Companion.MsoMdocFormatFilter


Unused import.

And I also noticed, that the two fields MsoMdocFormatFilter and SdJwtFormatFilter in the CredentialConfigurationFilter are probably unused and can be removed.

FelixRedAndroid · 2024-08-14T11:02:20Z

wallet-core/src/main/java/eu/europa/ec/eudi/wallet/issue/openid4vci/OfferCreator.kt

+//            val credentialConfigurationId =
+//                credentialIssuerMetadata.credentialConfigurationsSupported.filterValues { conf ->
+//                    credentialConfigurationFilter(conf)
+//                }.keys.firstOrNull() ?: throw IllegalStateException("No suitable configuration found")


Uncommented code, remove?

Still uncommented.

Hab einen Kommentar drüber damit man weiß, dass das wieder rein muss

FelixRedAndroid · 2024-08-14T11:06:27Z

wallet-core/src/main/java/eu/europa/ec/eudi/wallet/transfer/openid4vp/OpenId4vpManager.kt

+//        logger?.d(TAG, "Device Response to send (hex): ${Hex.toHexString(deviceResponse)}")
+//        logger?.d(TAG, "Device Response to send (cbor): ${CBOR.cborPrettyPrint(deviceResponse)}")


Uncommented code, remove?

FelixRedAndroid · 2024-08-14T11:07:25Z

wallet-core/src/main/java/eu/europa/ec/eudi/wallet/transfer/openid4vp/OpenId4vpManager.kt

@@ -363,7 +438,7 @@ class OpenId4vpManager(
        val responseUri =
            (this.responseMode as ResponseMode.DirectPostJwt?)?.responseURI?.toString()
                ?: ""
-        val nonce = this.nonce
+        val nonce = this.nonce //TODO MAYBE THIS NONCE


Here's a TODO. Is it still valid?

FelixRedAndroid · 2024-08-14T11:13:38Z

...a/ec/eudi/wallet/transfer/openid4vp/responseGenerator/OpenId4VpSdJwtResponseGeneratorImpl.kt

+        val key = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.R) {
+            KeyGeneratorImpl.getSigningKey(KeyGenerator.SigningKeyConfig(KeyProperties.AUTH_DEVICE_CREDENTIAL, 60))
+        } else {
+            throw Exception()


Maybe adjust to something like this?
return@runBlocking ResponseResult.Failure(AndroidException("Build version to low."))

FelixRedAndroid · 2024-08-14T11:14:34Z

...a/ec/eudi/wallet/transfer/openid4vp/responseGenerator/OpenId4VpSdJwtResponseGeneratorImpl.kt

+            throw IllegalArgumentException()
+        }
+
+        val namespace = "eu.europa.ec.eudi.pid.1"


This could be a constant.

FelixRedAndroid · 2024-08-14T11:19:05Z

...a/ec/eudi/wallet/transfer/openid4vp/responseGenerator/OpenId4VpSdJwtResponseGeneratorImpl.kt

+        val headerString = credentials.split(".").first()
+        val headerJson = JSONObject(String(Base64.getUrlDecoder().decode(headerString)))
+        val keyString = headerJson.getJSONArray("x5c").getString(0).replace("\n", "")
+        println(keyString)
+
+        val pemString = "-----BEGIN CERTIFICATE-----\n" +
+                        "${keyString}\n" +
+                        "-----END CERTIFICATE-----"
+
+        val certificateFactory: CertificateFactory = CertificateFactory.getInstance("X.509")
+        val certificate =
+            certificateFactory.generateCertificate(ByteArrayInputStream(pemString.toByteArray())) as X509Certificate
+
+        val ecKey = ECKey.parse(certificate)
+        val jwtSignatureVerifier = ECDSAVerifier(ecKey).asJwtVerifier()


I feel like I've seen this code part before in this PR... ^^

FelixRedAndroid

Did a rough review. Focused on code style / cleanup.
Looks good already, still left a few comments.

FelixRedAndroid · 2024-08-26T08:34:18Z

gradle/libs.versions.toml

@@ -21,17 +22,23 @@ junit = "4.13.2"
 junit-android = "1.1.5"
 kotlin = "1.8.10"
 ktor = "2.3.5"
+#ktorClientSerialization = "2.3.6"


Commented lines, remove?

FelixRedAndroid · 2024-08-26T08:34:22Z

gradle/libs.versions.toml

 eudi-lib-jvm-siop-openid4vp-kt = { module = "eu.europa.ec.eudi:eudi-lib-jvm-siop-openid4vp-kt", version.ref = "eudi-lib-jvm-siop-openid4vp-kt" }
 identity-credential = { module = "com.android.identity:identity-credential", version.ref = "identity-credential" }
 json = { module = "org.json:json", version.ref = "json" }
 junit = { module = "junit:junit", version.ref = "junit" }
 junit-jupiter-params = { module = "org.junit.jupiter:junit-jupiter-params", version.ref = "junit" }
 ktor-client-android = { module = "io.ktor:ktor-client-android", version.ref = "ktor" }
 ktor-client-logging = { module = "io.ktor:ktor-client-logging", version.ref = "ktor" }
+#ktor-client-serialization = { module = "io.ktor:ktor-client-serialization", version.ref = "ktorClientSerialization" }


Commented lines, remove?

FelixRedAndroid · 2024-08-26T08:34:29Z

wallet-core/build.gradle

@@ -149,6 +149,7 @@ dependencies {

    // Ktor Android Engine
    runtimeOnly libs.ktor.client.android
+//    implementation libs.ktor.client.serialization


Commented lines, remove?

FelixRedAndroid · 2024-08-26T08:35:41Z

wallet-core/src/main/java/eu/europa/ec/eudi/wallet/EudiWallet.kt

@@ -197,7 +200,9 @@ object EudiWallet {
     * @throws IllegalStateException if [EudiWallet] is not firstly initialized via the [init] method
     */
    fun deleteDocumentById(documentId: DocumentId): DeleteDocumentResult =
-        documentManager.deleteDocumentById(documentId)
+        documentManager.deleteDocumentById(documentId).apply {
+            if (this is DeleteDocumentResult.Success) DocumentManagerSdJwt.deleteDocument(documentId)


Kurze Frage:
Waren unsere ID's jetzt gesynced, so dass das funktionieren kann?
(Oder nicht und wir müssen das noch einmal anpassen?)

Die Id´s sind nicht synced, aber wir gehen davon aus, dass wir nur ein SdJwt haben können und löschen das wenn irgendein Mdoc gelöscht wird. Das ist good enough würde ich sagen. Wir sollten in die Lib noch einen Check einbauen, dass man nicht mehr als ein Pid haben kann, damit hier nichts kaputt gehen kann

FelixRedAndroid · 2024-08-26T09:12:02Z

wallet-core/src/main/java/eu/europa/ec/eudi/wallet/issue/openid4vci/DefaultOffer.kt

+            //.filterKeys { it in credentialOffer.credentialConfigurationIdentifiers }
+            //.filterValues { credentialConfigurationFilter(it) }


Still uncommented.

FelixRedAndroid · 2024-08-26T09:13:47Z

wallet-core/src/main/java/eu/europa/ec/eudi/wallet/issue/openid4vci/OfferCreator.kt

+//            val credentialConfigurationId =
+//                credentialIssuerMetadata.credentialConfigurationsSupported.filterValues { conf ->
+//                    credentialConfigurationFilter(conf)
+//                }.keys.firstOrNull() ?: throw IllegalStateException("No suitable configuration found")


Still uncommented.

FelixRedAndroid · 2024-08-26T09:15:01Z

wallet-core/src/main/java/eu/europa/ec/eudi/wallet/issue/openid4vci/ProcessResponse.kt

+                    } else {
+                        val cborBytes = Base64.getUrlDecoder().decode(credential.credential)
+
+                        logger?.d(TAG, "CBOR bytes: ${Hex.toHexString(cborBytes)}")


It's only debug level, but I think we should remove this logger here.

Der logger ist aus der originalen lib selbst, den haben wir nicht selbst hinzugefügt, aber ich kann ihn entfernen

FelixRedAndroid · 2024-08-26T09:15:49Z

wallet-core/src/main/java/eu/europa/ec/eudi/wallet/keystore/KeyGenerator.kt

+        return entry
+    }
+
+    private const val SIGNATURE_ALGORITHM = "SHA256withECDSA"


Maybe move up (line 23/24) to the other constants in this file?

FelixRedAndroid · 2024-08-26T09:16:48Z

wallet-core/src/main/java/eu/europa/ec/eudi/wallet/transfer/openid4vp/OpenId4vpManager.kt

+        val vpToken =
+            Base64.getUrlEncoder().withoutPadding()
+                .encodeToString(deviceResponse)
+        logger?.d(TAG, "VpToken: $vpToken")


Should this really be logged?

Der logger ist aus der originalen lib selbst, den haben wir nicht selbst hinzugefügt

Add sdJwt presentation

b2774ed

Pflanzmann requested a review from FelixRedAndroid August 14, 2024 07:41

FelixRedAndroid suggested changes Aug 14, 2024

View reviewed changes

Pflanzmann added 3 commits August 14, 2024 14:22

Fix requested changes

f355664

Fix delete sdJwt document from mdoc storage

500c785

Fix mdoc presentation

8f8f73e

Pflanzmann force-pushed the sdJwt2 branch 8 times, most recently from e1a417c to fe3a469 Compare August 19, 2024 14:27

Pflanzmann force-pushed the sdJwt2 branch from 31befec to 69a512d Compare August 21, 2024 15:33

Add zkp

df11201

Pflanzmann force-pushed the sdJwt2 branch from 9e858b9 to df11201 Compare August 23, 2024 14:34

Pflanzmann added 2 commits August 23, 2024 16:53

Fix sdjwt zkp

8676c49

Cleanup

c9d2e6f

Pflanzmann requested a review from FelixRedAndroid August 26, 2024 08:32

FelixRedAndroid suggested changes Aug 26, 2024

View reviewed changes

Change reviewer remarks

143dea7

Pflanzmann requested a review from FelixRedAndroid August 26, 2024 09:33

FelixRedAndroid approved these changes Aug 26, 2024

View reviewed changes

Pflanzmann merged commit 143dea7 into workingBranch Aug 26, 2024
9 of 14 checks passed

Pflanzmann deleted the sdJwt2 branch August 26, 2024 09:37

		//.filterKeys { it in credentialOffer.credentialConfigurationIdentifiers }
		//.filterValues { credentialConfigurationFilter(it) }

		@@ -20,6 +20,7 @@ import eu.europa.ec.eudi.openid4vci.CredentialOfferRequestResolver
		import eu.europa.ec.eudi.wallet.issue.openid4vci.CredentialConfigurationFilter.Companion.Compose
		import eu.europa.ec.eudi.wallet.issue.openid4vci.CredentialConfigurationFilter.Companion.MsoMdocFormatFilter

		// logger?.d(TAG, "Device Response to send (hex): ${Hex.toHexString(deviceResponse)}")
		// logger?.d(TAG, "Device Response to send (cbor): ${CBOR.cborPrettyPrint(deviceResponse)}")

Add sdJwt presentation #4

Add sdJwt presentation #4

Uh oh!

Conversation

Pflanzmann commented Aug 14, 2024

Uh oh!

FelixRedAndroid left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

FelixRedAndroid left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!